add crucible to propolis-standalone config toml #344
Conversation
bin/propolis-standalone/Cargo.toml
Outdated
| @@ -17,14 +17,17 @@ libc.workspace = true | |||
| toml.workspace = true | |||
| tokio = { workspace = true, features = ["io-util", "rt-multi-thread"] } | |||
| serde = { workspace = true, features = ["derive"] } | |||
| propolis.workspace = true | |||
| propolis = { workspace = true, features = ["crucible-full"], default-features = false } | |||
There was a problem hiding this comment.
I'd like to keep crucible as an optional dependency for those not using it in their standalone work.
There was a problem hiding this comment.
ah, my brain missed the feature flags at the bottom of the toml. can do
| "crucible" => { | ||
| info!(log, "Building a crucible VolumeConstructionRequest from options {:?}", be.options); |
There was a problem hiding this comment.
Assuming the crucible feature is made optional, move this into a function gated with a cfg() conditional?
You might have already seen this, but this sounds like oxidecomputer/crucible#660 to me. I think the final verdict there was that this is caused by a crucible version mismatch. It would be interesting to know if that wasn't the case here. |
There was a problem hiding this comment.
I'm curious if you explored how easy it would be to add support for a crucible backend in propolis-server. Since propolis-server is what we're running in prod, and the TOML file is not the way propolis-server will be configured in prod, we might have to a bit more careful about it, but I think that's true for all things in the TOML file.
If it seems like too much of a lift to add this to propolis-server as well, I would like to add least add a pointer to the readme on instructions getting going there (docs/migrate-with-crucible.md)
README.md
Outdated
| @@ -157,6 +157,72 @@ which acts as a serial port. One such tool for accessing this serial port is | |||
| [sercons](https://github.com/jclulow/vmware-sercons), though others (such as | |||
| "screen") would also work. | |||
|
|
|||
| propolis-standalone also supports defining crucible-backed storage devices, | |||
| thought it is somewhat inconvenient to do so without scripting. Currently you can only use a single region per block device. Read the comments in this TOML example for more details: | |||
There was a problem hiding this comment.
probably worth linking to relevant crucible docs here in case any of documented values here change
yeah i was experiencing it with specifically a version mismatch |
actually i also get this if i just never turn on downstairs. upstairs tries to connect forever so propolis-standalone never fully shuts down. |
faithanalog
force-pushed
the
artemis/add-crucible-to-standalone
branch
from
daa9496
to
346b588
Compare
There was a problem hiding this comment.
Looks fine to me, if it's working for you as desired.
The config parsing is rather brittle (with all the unwraps and whatnot) if the user provides an imperfect configuration, but given that propolis-standalone is a developer playground, that's par for the course. Having the example in the README should be helpful on that front.
Please squash your commits down prior to integration.
|
Integrated via #486 |
this adds basic support for configuring propolis-standalone to use Crucible. I'm not super familiar with propolis internals; let me know if anything needs adjusting.
There's currently an issue with it where if there's a connection problem between crucible upstairs/downstairs, upstairs will get stuck in a reconnection loop, blocking the VM from exiting in response to Ctrl-C or shutdown. It's definitely something just blocking on waiting for all the async jobs to complete, which they never do. Things exit normally if the connection doesn't have any problems before the exit.
i don't like that it does that, but I'm not really sure how to fix it. for the main thing i want to use this in, it doesnt matter and i can just
kill -9.